Skip to content

fix: make the pre-commit API key check work on macOS and cover every staged file - #2466

Open
kikoso wants to merge 1 commit into
mainfrom
fix/pre-commit-api-key-check
Open

kikoso wants to merge 1 commit into
mainfrom
fix/pre-commit-api-key-check

Conversation

@kikoso

@kikoso kikoso commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

The sample hooks/pre-commit never rejected anything on macOS. The character class [0-9A-Za-z\-_] is an invalid range for the BSD grep shipped with macOS, so grep fails with invalid character range, exits with 2, and the hook takes it as "no match" and lets the commit through. GNU grep accepts it, which is probably why it went unnoticed. The same hook has been copied to other repos (i.e. android-maps-compose), so they inherit the issue.

Changes:

  • Regex moved to [0-9A-Za-z_-]{35}, which is portable between BSD and GNU grep.
  • The hook now checks the staged changes (git diff --cached) instead of the working copy, so it looks at what is actually going to be committed.
  • It checks every staged file, not only local.defaults.properties, since a key pasted in a Kotlin file or a README is the more realistic leak. Only added lines are checked, so a commit that removes a key still goes through.

Testing

Tested in a scratch repo with /usr/bin/grep on macOS:

  • Placeholder MAPS_API_KEY=YOUR_API_KEY: committed.
  • Real-looking key in a .properties file: rejected.
  • Real-looking key in a .kt file: rejected.
  • Commit removing a key: committed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant